Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NEW] Make channel/group delete call answer to roomName #6857

Merged

Conversation

reist
Copy link
Contributor

@reist reist commented May 2, 2017

A nice shortcut, also makes more sense with the returned error (The parameter "roomId" or "roomName" is required).

Would it make sense to update all channel/group calls to accept roomName? The only call that I think really won't play nice with this is rename - with both roomName and name parameters, it'll be confusing.

@reist reist force-pushed the channel-and-group-delete-by-name branch 6 times, most recently from ae0c03e to d8a37f3 Compare May 2, 2017 15:49
@engelgabriel engelgabriel requested a review from graywolf336 May 2, 2017 19:50
@graywolf336
Copy link
Contributor

If you do feel like going ahead and adding the roomName to each of the methods or find a better way to accomplish that, go for it! :)

@reist
Copy link
Contributor Author

reist commented May 11, 2017

@graywolf336 Alright, I have those changes ready. Should I add them to this request or create a new one?

@reist reist force-pushed the channel-and-group-delete-by-name branch 4 times, most recently from d5381e0 to 503cfb1 Compare May 14, 2017 17:06
reist added 2 commits May 16, 2017 00:25
In .rename, having both name and roomName parameters would be confusing.

Also:
Sync groups.close error message with channels.close
Sync and fix error message in groups.open that was using a parameter that
couldn't be there before this patch.
Extract the bodyParams/queryParams logic.
@reist reist force-pushed the channel-and-group-delete-by-name branch from 503cfb1 to eab7677 Compare May 15, 2017 21:25
@reist
Copy link
Contributor Author

reist commented May 16, 2017

Updated to change all calls except rename and made all the tests pass. Can this get a new review, please?

@rodrigok rodrigok merged commit eddb475 into RocketChat:develop May 17, 2017
@rodrigok rodrigok added this to the 0.57.0 milestone May 17, 2017
@reist reist deleted the channel-and-group-delete-by-name branch July 24, 2017 11:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants